Skip to content

Fix '--make-template' for union types #1760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Nov 8, 2022
Merged

Fix '--make-template' for union types #1760

merged 5 commits into from
Nov 8, 2022

Conversation

@tom-tan
Copy link
Member

@tom-tan tom-tan commented Nov 8, 2022

This request makes cwltool --make-template to support union types.

It includes the following fixes:

  • --make-template fails with the following message: "IndexError: string index out of range"
    • It is caused by calling CommentedSeq()#insert with empty comment. insert can handle None comment but cannot handle empty string comment. I fixed it by checking whether the comment is an empty string or not.
  • Even if the above issue is fixed, cwltool --make-template does not work with the union type because it try to generate examples for all possible types. This request fixes it by generating an example for the first candidate type

I checked it works with the following CWL document.

...
inputs:
  repo:
    type: string
    default: "ddbj"
    inputBinding:
      position: 1
      prefix: "-r"
  run_ids:
    type:
      - string
      - string[]
    inputBinding:
      position: 2
...
  • Before merging this request:
$ cwltool --debug --make-template download-sra.cwl 
INFO /home/vscode/.local/bin/cwltool 3.1.20221108103845
INFO Resolved 'download-sra.cwl' to 'file:///workspaces/cwltool/download-sra.cwl'
ERROR I'm sorry, I couldn't load this CWL file.
The error was: 
Traceback (most recent call last):
  File "/home/vscode/.local/lib/python3.10/site-packages/cwltool/main.py", line 1171, in main
    make_template(tool)
  File "/home/vscode/.local/lib/python3.10/site-packages/cwltool/main.py", line 779, in make_template
    generate_input_template(tool),
  File "/home/vscode/.local/lib/python3.10/site-packages/cwltool/main.py", line 336, in generate_input_template
    template.insert(0, name, value, comment)
  File "/home/vscode/.local/lib/python3.10/site-packages/ruamel/yaml/comments.py", line 896, in insert
    self.yaml_add_eol_comment(comment, key=key)
  File "/home/vscode/.local/lib/python3.10/site-packages/ruamel/yaml/comments.py", line 434, in yaml_add_eol_comment
    if comment[0] != '#':
IndexError: string index out of range
  • After merging this request:
$ cwltool --make-template download-sra.cwl 
INFO /home/vscode/.local/bin/cwltool 3.1.20221108103845
INFO Resolved 'download-sra.cwl' to 'file:///workspaces/cwltool/download-sra.cwl'
run_ids: a_string  # one of type "string", array of type "string"
repo: "ddbj"  # default value of type "string".

I guess this request can fix #1404.

mr-c
mr-c requested changes Nov 8, 2022
View reviewed changes
Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Please run make cleanup

@codecov
Copy link

codecov bot commented Nov 8, 2022 •
edited
Loading

Codecov Report

Merging #1760 (d453175) into main (14f402f) will increase coverage by 1.50%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             main    #1760      +/-   ##
==========================================
+ Coverage   80.89%   82.40%   +1.50%     
==========================================
  Files          47       47              
  Lines        8177     8179       +2     
  Branches     2230     2231       +1     
==========================================
+ Hits         6615     6740     +125     
+ Misses       1047      944     -103     
+ Partials      515      495      -20
Impacted Files Coverage Δ
cwltool/main.py 75.46% <85.71%> (+1.39%) ⬆️
cwltool/docker.py 71.42% <0.00%> (-0.85%) ⬇️
cwltool/job.py 80.67% <0.00%> (-0.40%) ⬇️
cwltool/process.py 92.47% <0.00%> (+0.61%) ⬆️
cwltool/checker.py 96.88% <0.00%> (+1.55%) ⬆️
cwltool/singularity.py 65.80% <0.00%> (+1.73%) ⬆️
cwltool/command_line_tool.py 86.92% <0.00%> (+2.31%) ⬆️
cwltool/workflow.py 91.79% <0.00%> (+2.34%) ⬆️
cwltool/workflow_job.py 86.59% <0.00%> (+3.53%) ⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tom-tan
Copy link
Member Author

tom-tan commented Nov 8, 2022

Thank you for the suggestion.
I run it but it does not change main.py.

$ make cleanup
...
reformatted mypy-stubs/prov/model.pyi

All done! ✨ 🍰 ✨
1 file reformatted, 413 files left unchanged.
...
diff-quality --compare-branch=main --violations=pydocstyle --fail-under=100 pydocstyle_report.txt
-------------
Diff Quality
Quality Report: pydocstyle
Diff: main...HEAD, staged and unstaged changes
-------------
No lines with quality information in this diff.
-------------

mr-c
mr-c approved these changes Nov 8, 2022
View reviewed changes
Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@mr-c mr-c merged commit 6af9320 into main Nov 8, 2022
@mr-c mr-c deleted the support-template-for-union branch November 8, 2022 20:51
@tom-tan
Copy link
Member Author

tom-tan commented Nov 8, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mr-c mr-c mr-c approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Creating input yaml - issue with ruamel comments.py

3 participants

@tom-tan @mr-c